Skip to content

feat(button): Busy spinner with absolute position#110202

Open
TkDodo wants to merge 9 commits intomasterfrom
tkdodo/feat/de-980-busy-state
Open

feat(button): Busy spinner with absolute position#110202
TkDodo wants to merge 9 commits intomasterfrom
tkdodo/feat/de-980-busy-state

Conversation

@TkDodo
Copy link
Collaborator

@TkDodo TkDodo commented Mar 9, 2026

this is the 3rd attempt to land de-980 after two reverts. This approach does not change the button internal layout to avoid the large blast radius of the first two approaches.

@linear-code
Copy link

linear-code bot commented Mar 9, 2026

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 9, 2026
Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conceptually approved! Didn't see any visual errors in the preview 🙌

Would love to see the button docs updated and another once-over on the aria states for the entire component.

@TkDodo TkDodo marked this pull request as ready for review March 10, 2026 08:26
@TkDodo TkDodo requested a review from a team as a code owner March 10, 2026 08:26
@TkDodo
Copy link
Collaborator Author

TkDodo commented Mar 10, 2026

when all buttons are busy 😂

Screenshot 2026-03-10 at 09 30 10

Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Props spread overrides busy aria-label for icon-only buttons
    • Moved aria-label assignment after {...props} spread to ensure the 'Busy' label is properly applied when buttons are in busy state, following the same pattern as onClick and role.

Create PR

Or push these changes by commenting:

@cursor push 453fa0dee7
Preview (453fa0dee7)
diff --git a/static/app/components/core/button/button.tsx b/static/app/components/core/button/button.tsx
--- a/static/app/components/core/button/button.tsx
+++ b/static/app/components/core/button/button.tsx
@@ -38,7 +38,6 @@
       disabled={!tooltipProps?.title}
     >
       <StyledButton
-        aria-label={busy ? 'Busy' : accessibleLabel}
         aria-disabled={disabled}
         aria-busy={busy}
         disabled={disabled}
@@ -46,6 +45,7 @@
         type={type}
         busy={busy}
         {...props}
+        aria-label={busy ? 'Busy' : accessibleLabel}
         onClick={handleClick}
         role="button"
       >
This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

@JonasBa
Copy link
Member

JonasBa commented Mar 10, 2026

Looks good :) @natemoo-re should we make the loader color blue here, wdyt?

@natemoo-re
Copy link
Member

@JonasBa I don't mind currentColor to avoid maintaining a separate table for spinner color variants!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants